fix: improve buffer cleanup and rename in gin-blame #167
Conversation
|
Warning Rate limit exceeded@lambdalisue has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughReplaces name-based buffer tracking with per-buffer stored buffer numbers for blame nav/detail/file buffers; switches renaming from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Cmd as blame/edit.ts
participant NavBuf as NavBuffer
participant DetailBuf as DetailBuffer
participant FileBuf as FileBuffer
Note over Cmd: Begin blame-edit flow
User->>Cmd: open blame edit
Cmd->>NavBuf: getbufvar(gin_blame_paired_bufnr_nav)
Cmd->>DetailBuf: getbufvar(gin_blame_paired_bufnr_detail)
Cmd->>FileBuf: ensure file buffer & bufnr
alt rename required
Cmd->>FileBuf: execute `noautocmd file <new-name>`
FileBuf-->>Cmd: buffer updated (actual bufnr)
Cmd->>Cmd: store bufnrs on nav/detail/file buffers
end
Cmd->>NavBuf: update signs/mappings using stored bufnr
Cmd->>DetailBuf: update signs/mappings using stored bufnr
Cmd->>Cmd: register BufUnload autocmds that read paired bufnrs via getbufvar
Note over Cmd: switchToCommit completes
Cmd->>Cmd: call `redraw`
User->>Cmd: close or cleanup
Cmd->>NavBuf: cleanup using stored bufnr
Cmd->>DetailBuf: cleanup using stored bufnr
Cmd->>FileBuf: cleanup using stored bufnr
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #167 +/- ##
=======================================
Coverage 61.59% 61.59%
=======================================
Files 21 21
Lines 1156 1156
Branches 163 163
=======================================
Hits 712 712
Misses 443 443
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Use buffer numbers instead of buffer names for cleanup autocmd This prevents issues when buffer names contain special characters - Replace nvim_buf_set_name with :file command for better Vim/Neovim compatibility - Remove unnecessary buffer switching logic after rename - Store paired buffer numbers in buffer variables for reliable cleanup
lambdalisue
force-pushed
the
fix/blame-buffer-cleanup
branch
from
125f12d to
430720f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
denops/gin/command/blame/edit.ts(7 hunks)
🧰 Additional context used
🪛 GitHub Actions: Test
denops/gin/command/blame/edit.ts
[error] 323-328: deno fmt --check failed. Found 1 not formatted file in 124 files.
🔇 Additional comments (4)
denops/gin/command/blame/edit.ts (4)
319-324: Good: store paired buffer numbers on the ginblame buffer.This enables reliable cleanup independent of buffer names. LGTM.
355-358: Good: nav buffer stores paired bufnrs for cleanup.Symmetric with ginblame; consistent and reliable. LGTM.
396-399: Good: detail buffer stores paired bufnrs.Completes the triangle for robust cleanup. LGTM.
296-296: <!-- [scratchpad_start]
[task verification_formatter_issue 3/3]
Gathered git diff information and analyzed the changes.[observations]
- Git diff shows this PR changed the autocmd strings from multi-line template literals with bufnr() calls to single-line strings using getbufvar(str2nr(expand()...))
- OLD code: Multi-line with
bufnr('${bufnameNav.replaceAll("'", "''")}')- NEW code: Single-line with
getbufvar(str2nr(expand('<abuf>')), 'gin_blame_paired_bufnr_nav')- Current file state shows lines 296, 377, 401 ARE indeed single-line strings (264-267 characters)
- No uncommitted changes in working directory (git diff shows 0 lines)
- biome.json is untracked in repo (possible formatter config)
[analysis]
- The refactoring itself (template literals → getbufvar) has already been applied
- These ARE long lines by standard metrics (>260 chars vs typical 80-120 limit)
- Review comment claims "diffs above also improve fmt stability" but the NEW state still has long lines
- Original concern was formatter failure due to extremely long strings
- Without Deno formatter available, cannot verify if code passes/fails formatting checks
- The code IS different/improved from before but may still benefit from line breaks
[pending]
- Cannot resolve: deno fmt not available in sandbox
- Cannot confirm: whether current state passes formatter checks
- Cannot test: if formatter would break lines
[actions]
- Flag as verification needed since formatter unavailable
- Recommend manual verification by developer
- Note that code has been changed but line lengths still exceed typical limits
[done]
Verification complete - formatter tool unavailable, cannot conclusively verify formatting status. Code has been refactored but line lengths remain long (264-267 chars).[scratchpad_start] -->
Unable to verify formatter status—tool unavailable in sandbox.
The code has been refactored from multi-line template literals to single-line
getbufvar()calls (lines 296, 377, 401), but these lines remain long (264–267 characters). The Deno formatter is unavailable to verify formatting compliance.Please run locally:
deno fmt --check deno fmtConfirm the formatter passes without errors on these lines.
lambdalisue
force-pushed
the
fix/blame-buffer-cleanup
branch
3 times, most recently
from
2b35b92 to
6f38d9f
Compare
Redraw the display after switching commits (Enter key) and navigating history (Ctrl-O/Ctrl-I) to ensure the buffer and cursor changes are immediately visible to the user.
lambdalisue
force-pushed
the
fix/blame-buffer-cleanup
branch
from
6f38d9f to
714dc46
Compare
Summary
nvim_buf_set_namewith:filecommand for better Vim/Neovim compatibilityTest plan
Summary by CodeRabbit